Conversation
| @@ -37,9 +37,7 @@ if (sharp && path.startsWith('@img/sharp-linux-x64') && !sharp._isUsingX64V2()) | |||
| sharp = null; | |||
| } | |||
|
|
|||
| if (sharp) { | |||
| module.exports = sharp; | |||
There was a problem hiding this comment.
Conditional exports are not supported by ESM but it doesn't matter here because importing the module will throw as needed
|
This is great, thank you for doing the hard work Florian, and at less of an impact in terms of changed code than I had imagined, merci donc d'avoir fait simple. For the v0.35.x releases I'd like to maintain compatibility with versions of Node.js where ESM isn't fully available and then I suspect the v0.36.x releases (or maybe v1.0.x if I'm feeling lucky after 13 years) is the place for this to land and wish CommonJS bon voyage. As an aside, I've been invited to (and accepted) the Astro event in London in a few weeks and spotted your name on the list so I might be able to thank you in person for this. |
|
Glad I can help :) Yeah I saw you on the list for the event, looking forward to meeting you! I'll give a talk on fonts Btw I was thinking about this PR during the weekend and I don't know if the packages under |
|
@lovell can you approve the workflow to see if the tests pass? |
|
To keep this PR as minimal as possible, I switched from ESM first + CJS compat to CJS first + ESM compat, which required a few more changes. Workflow should run better next time you approve it! |
|
Build and tests are now passing locally! |
| ], | ||
| "scripts": { | ||
| "build": "node install/build.js", | ||
| "build:dist": "node scripts/build.mjs", |
There was a problem hiding this comment.
I had to add another script for lib to dist compilation because if I understood correctly, sharp can be built from source by doing npm explore sharp -- npm run build. If the compilation happened in this script, it would fail since mjs files in lib are not shipped to NPM anymore
|
I swear this should work now |
2 participants
Closes #2981
I kept this PR as minimal as I could. It introduces esbuild to be able to compile ESM to CJS to support both:
libto ESM because esbuild doesn't support transformingrequire()intoimportrequire()Things left to do where I'll need some help:
install/build.jsrequire()calls inlibvips.jsbecause they only seem consumed bybinding.gyp